Skip to content

Conversation

@linzhp
Copy link
Contributor

@linzhp linzhp commented May 20, 2025

What type of PR is this?
Feature

What does this PR do? Why is it needed?
After #4268, nogo collects facts from all targets. So go_register_nogo's includes and excludes now overlap with the JSON config's only_files and exclude_files, which is to surppress nogo errors in certain files/targets. Meanwhile, collecting facts can be slow for very large Go packages for some analyzers, slower than compilation. To address the performance issue, this PR revert excludes to its previous behavior: stop nogo from collecting facts from certain packages. To avoid collecting facts from IDL-generated code, users can do this:

    go_register_nogo(
        nogo = "@//:default_nogo",
        excludes = ["@//idl:__subpackages__"],
    )

The includes of go_register_nogo a no-op with a warning message, so nogo collects facts from all packages unless excluded explicitly. Users have to use the JSON config to limit the scope of nogo validation.

This PR also exclude all external targets from nogo validation by default, which is equivalent to

{
  "_base": {
    "exclude_files": {
      "^external/": "external repositories"
    }
  }
}

Other notes for review
Alternatively, to be backward-compatible, we can add a flag or attribute like exclude_collect_facts, but adding that on top of excludes and exclude_files makes hard to remember which is which.

}
if currentConfig.excludeFiles == nil {
// TODO(linzhp): add a test to verify the case of [].
currentConfig.excludeFiles = []*regexp.Regexp{_externalFilePattern}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this, most repos no longer need the JSON config unless they want to exclude certain paths in the repo. @fmeum

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant